Fenrir fixes (2026-06-23)#70
Conversation
…-3481) The DTLS branch called bind_socket.recvfrom(1) before creating the context. On UDP that removes the entire first datagram (the client's ClientHello) from the queue and discards everything past the first byte, so wolfSSL_accept() had nothing to consume and the handshake only recovered after the client's retransmit timer. The captured from_addr was also reused for every iteration of the -i loop. Replace it with a peek_peer_address() helper that uses MSG_PEEK to read the source address without consuming the datagram, and move the peek into the accept loop so the address is refreshed per connection.
wolfSSL_write can return WOLFSSL_ERROR_WANT_READ (e.g. during a renegotiation that must read a record before progressing; secure renegotiation is enabled by default). write() only handled WANT_WRITE, so WANT_READ fell through to a generic SSLError and non-blocking callers tore the session down. Add a WANT_READ branch raising SSLWantReadError, matching do_handshake().
wolfSSL_read can return WOLFSSL_ERROR_WANT_WRITE when the SSL layer must flush a handshake record (e.g. renegotiation) before returning data. read() only handled WANT_READ, raising a generic SSLError otherwise, which stops non-blocking callers from select()-ing on writability. Add a WANT_WRITE branch raising SSLWantWriteError.
recv_into() shares read()'s error-mapping pattern and inherited the same omission: wolfSSL_read returning WOLFSSL_ERROR_WANT_WRITE (during a renegotiation needing a write) was reported as a generic SSLError instead of SSLWantWriteError, breaking non-blocking callers that distinguish readiness directions. Add the WANT_WRITE branch.
For DTLS, write()/read()/recv_into() called do_handshake() on every call. do_handshake() runs wolfSSL_accept/connect, which on a non-blocking socket can raise SSLWantReadError and abort an I/O long after the handshake finished, and made DTLS write-side behaviour inconsistent with TCP. Track completion with a _handshake_complete flag set on a successful do_handshake(), and only drive the handshake from I/O methods while that flag is False.
There was a problem hiding this comment.
Pull request overview
This PR bundles several Fenrir-tracked behavioral fixes to the wolfSSL Python bindings to better match CPython ssl semantics (peer cert handling, I/O error mapping, DTLS handshake behavior) and to harden the shipped client/server examples (hostname verification + DTLS UDP peeking), with accompanying regression tests.
Changes:
- Adjust peer-certificate retrieval so
getpeercert()/get_peer_x509()returnNonewhen the peer presented no certificate. - Improve
SSLSocketI/O behavior: accept bytes-like inputs forwrite(), mapWANT_READ/WANT_WRITEto the corresponding exceptions, and avoid re-driving DTLS handshakes after completion. - Update examples (client hostname verification, DTLS server peek) and add targeted pytest coverage for each fix.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
wolfssl/__init__.py |
Core binding behavior changes: peer cert retrieval, bytes-like write(), DTLS handshake completion tracking, WANT_* exception mapping. |
examples/client.py |
Adds hostname-verification behavior (and opt-out flag) and passes server_hostname to wrap_socket(). |
examples/server.py |
Fixes DTLS UDP example to peek source address without consuming the ClientHello datagram. |
tests/test_write_bytes.py |
Regression tests ensuring write() sends bytes-like contents verbatim and rejects non-bytes-like inputs. |
tests/test_io_error_mapping.py |
Verifies WANT_READ/WANT_WRITE error-code mapping to SSLWantReadError/SSLWantWriteError. |
tests/test_getpeercert.py |
Confirms getpeercert()/get_peer_x509() return None when the peer provides no certificate. |
tests/test_dtls_server_example.py |
Ensures DTLS server peeking doesn’t consume the datagram needed for handshake. |
tests/test_dtls_handshake_once.py |
Ensures DTLS handshake is driven only until completion (not on every I/O call). |
tests/test_client_example.py |
Validates example client verification/hostname-check configuration behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The client example set CERT_REQUIRED and loaded CA roots but never set check_hostname or passed server_hostname to wrap_socket, so wolfSSL validated the chain to a trusted CA without binding the certificate to the requested host. A peer presenting any CA-trusted certificate for a different hostname would be accepted by anyone reusing this as a secure client template. Make verification configure hostname checking by default (via a new configure_verification helper) and add a -n flag to opt out explicitly for IP literals or test certificates.
write() converted data with t2b(), which str()-encodes anything that is
not already bytes. Valid bytes-like inputs such as bytearray and
memoryview were transmitted as their Python repr ("bytearray(b'...')",
"<memory at ...>") instead of their contents, corrupting the stream.
Convert via the buffer protocol (bytes(memoryview(data))) and raise
TypeError for objects that are not bytes-like, matching the stdlib ssl
module.
get_peer_x509() checked only whether the session was NULL and then built a WolfSSLX509, whose __init__ called wolfSSL_get_peer_certificate() and raised SSLError on NULL. On a valid connection where the peer presented no certificate (e.g. a server not requesting a client cert), this raised instead of returning None as the stdlib ssl getpeercert() contract requires. Fetch the certificate in get_peer_x509(), return None when it is NULL, and have WolfSSLX509 wrap the already-obtained pointer.
43dc9f5 to
93954c9
Compare
dgarske
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: reviewOverall recommendation: COMMENT
Findings: 3 total — 3 posted, 0 skipped
3 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Medium] Client example default host is an IP literal but hostname verification is now on by default —
examples/client.py:46-48,134-158,183-191 - [Low] Non-ASCII character in test source literal —
tests/test_write_bytes.py:80-84 - [Low] WolfSSLX509 type discrimination relies on exact cffi cname string match —
wolfssl/__init__.py:99-111
Review generated by Skoll
| help="Disable client cert check" | ||
| ) | ||
|
|
||
| parser.add_argument( |
There was a problem hiding this comment.
🟠 [Medium] Client example default host is an IP literal but hostname verification is now on by default
F-5621 enables hostname verification whenever cert checking is on (the default). But the client's default host is the IP literal -h 127.0.0.1 (line 46), and configure_verification returns args.h as server_hostname and sets check_hostname = True for that default. __init__ then calls wolfSSL_check_domain_name(ssl, "127.0.0.1"). wolfSSL_check_domain_name performs DNS/CN-style domain matching; the bundled certs/server-cert.pem carries 127.0.0.1 only as an IP Address SAN (SAN is DNS:example.com, IP Address:127.0.0.1), not a DNS-type entry. If wolfSSL's domain-name check does not match IP-type SANs, the out-of-the-box python client.py run will now fail with a hostname/domain mismatch and require the new -n flag — a behavior change for the example's default invocation. The function's own docstring even states IP literals need -n, which is exactly the default host. Additionally, sending an IP literal via SNI (use_sni("127.0.0.1")) is not RFC-6066 conformant.
Fix: Confirm that a default python client.py (host 127.0.0.1) still completes the handshake against the bundled certs with hostname verification enabled. If wolfSSL_check_domain_name does not match the IP-type SAN, either auto-disable the hostname check for IP-literal hosts, document that -n is required for the default host, or change the example default host to a DNS name in the cert SAN.
|
|
||
|
|
||
| def test_write_str_is_utf8_encoded(monkeypatch): | ||
| # Backward compatibility: str is UTF-8 encoded (historical t2b() |
There was a problem hiding this comment.
🔵 [Low] Non-ASCII character in test source literal
The test embeds a raw non-ASCII character é (U+00E9) in the source to verify UTF-8 encoding. The file declares # -*- coding: utf-8 -*- so it is functionally valid, but the wolfSSL project convention is 7-bit ASCII for committed code. The same assertion can be expressed with an ASCII-only escape while still exercising a multi-byte UTF-8 encoding.
Fix: Replace the literal é with the escape \u00e9 to keep the source 7-bit ASCII while testing the same multi-byte path.
| @@ -97,7 +97,15 @@ class WolfSSLX509(object): | |||
| """ | |||
|
|
|||
| def __init__(self, session): | |||
There was a problem hiding this comment.
🔵 [Low] WolfSSLX509 type discrimination relies on exact cffi cname string match
The constructor distinguishes a WOLFSSL* session from a WOLFSSL_X509* by comparing _ffi.typeof(session).cname == "WOLFSSL *". This is verified correct against the current cdef (wolfSSL_get_peer_certificate(WOLFSSL*) returns WOLFSSL_X509*), but it is brittle: it depends on cffi's exact rendering of the type name, will raise TypeError if a non-cdata is ever passed, and silently stores whatever is in the else branch without validating it is actually a WOLFSSL_X509*. A small comment is already present; a slightly more defensive check (e.g. comparing against the X509 type, or guarding the typeof call) would be more robust to future cdef changes.
Fix: Optional hardening: key the branch off the expected X509 type or guard the typeof lookup so the constructor degrades gracefully if an unexpected type is passed. Functionally correct as-is.
The bundled CRL expired 2024-11-11, so the client example's default CRL load (enabled unless -C) fails. Take the current CRL from wolfSSL v5.8.4-stable certs/crl/crl.pem: same Sawtooth CA and key, revokes only serial 02, valid until 2028-08-09. Note: the bundled ca/server/client certs expire 2026-09-08 and will need a refresh of their own before then.
Review follow-up for F-5621. wolfSSL_check_domain_name() only matches DNS names: on this path CheckForAltNames() is called with isIP=0, so iPAddress SANs are always skipped (verified on v5.8.4-stable and master). The default invocation (host 127.0.0.1) therefore failed the handshake with DOMAIN_NAME_MISMATCH (-322) once hostname verification was enabled by default. Skip the hostname check for IP literal hosts and say so, keeping CERT_REQUIRED verification. This also stops offering an IP literal in SNI, which RFC 6066 forbids. Connecting by DNS name still enables the hostname check. Add unit tests for the IP literal paths and an end-to-end test that runs server.py and client.py with default arguments.
Use the \u00e9 escape instead of a literal e-acute. The test still exercises multi-byte UTF-8 encoding.
Compare interned cffi type objects instead of rendered type name strings and raise TypeError for anything that is not a WOLFSSL* or WOLFSSL_X509*.
socket.inet_pton() is missing on some supported platforms (Python 2.7 on Windows). getaddrinfo() with AI_NUMERICHOST parses without resolving and is available everywhere. It also handles scoped IPv6 literals.
subprocess.run() is Python 3.5+. Use Popen with communicate() and a threading.Timer watchdog in place of the communicate() timeout, which is 3.3+.
This branch collects a set of Fenrir-tracked fixes for the wolfSSL Python bindings.
Changes
getpeercertwhen peer has no certificate (F-5623) — match the stdlibsslcontract.SSLSocket.write(F-5622) — avoid mangling bytes-like inputs.WANT_WRITEfromSSLSocket.recv_into()toSSLWantWriteError(F-3907).WANT_WRITEfromSSLSocket.read()toSSLWantWriteError(F-3906).WANT_READfromSSLSocket.write()toSSLWantReadError(F-3905).Each fix ships with accompanying tests.